Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/162/add wallet endpoints #239

Merged
merged 7 commits into from
Dec 17, 2022
Merged

Conversation

morrieinmaas
Copy link
Contributor

closes #162

Moriarty added 2 commits December 13, 2022 15:21
Signed-off-by: Moriarty <[email protected]>
TODO:
* refactor and rename a a few things

Signed-off-by: Moriarty <[email protected]>
@morrieinmaas
Copy link
Contributor Author

@blu3beri I need to go over a few things still mainly the naming of some vars and the help strings. I'll tag you when done, but feel free to leave comments otherwise (but be aware it's unfinished)

@berendsliedrecht
Copy link
Member

Thanks for the contribution @morrieinmaas! Was there a specific reason why you needed this, or just resolving some of the open issues?

@morrieinmaas
Copy link
Contributor Author

morrieinmaas commented Dec 13, 2022

Thanks for the contribution @morrieinmaas! Was there a specific reason why you needed this, or just resolving some of the open issues?

TBH a mix of reasons. as you (correctly) suggested closing open issues, wanted to not get rusty/rustier in Rust and train being rusty... other reasons... and also thought that since there is an intention to wrap the entire cloud agent eventually, which would be quite handy to have thought this was an alright step to take in that direction. Also was sat on a train for 2 hours the other day so I thought this was a nice pastime.

Signed-off-by: Moriarty <[email protected]>
@morrieinmaas morrieinmaas marked this pull request as ready for review December 13, 2022 19:40
Moriarty added 2 commits December 14, 2022 11:27
Signed-off-by: Moriarty <[email protected]>
Copy link
Member

@berendsliedrecht berendsliedrecht left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some small nit picks (I assume every endpoint and all options work).

I think it would be good to, somewhere in the future, do a small rewrite for modules. It is getting a bit messy everywhere and we are doing some odd things.

crates/agent/src/modules/wallet.rs Outdated Show resolved Hide resolved
crates/agent/src/modules/wallet.rs Outdated Show resolved Hide resolved
crates/agent/src/modules/wallet.rs Outdated Show resolved Hide resolved
crates/cli/src/help_strings.rs Outdated Show resolved Hide resolved
crates/cli/src/help_strings.rs Outdated Show resolved Hide resolved
crates/cli/src/help_strings.rs Outdated Show resolved Hide resolved
crates/cli/src/modules/wallet.rs Outdated Show resolved Hide resolved
crates/cli/src/modules/wallet.rs Outdated Show resolved Hide resolved
crates/cloudagent-python/src/cloudagent/wallet.rs Outdated Show resolved Hide resolved
crates/cloudagent-python/src/cloudagent/wallet.rs Outdated Show resolved Hide resolved
@morrieinmaas
Copy link
Contributor Author

Some small nit picks (I assume every endpoint and all options work).

I think it would be good to, somewhere in the future, do a small rewrite for modules. It is getting a bit messy everywhere and we are doing some odd things.

Sure, if you explain a bit what you have in mind I'm happy to help

@berendsliedrecht berendsliedrecht merged commit c563262 into main Dec 17, 2022
@berendsliedrecht berendsliedrecht deleted the feat/162/add-wallet-endpoints branch December 17, 2022 11:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for wallet operations
2 participants